-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initialization.rst: fix superscripts and render code as code #531
base: main
Are you sure you want to change the base?
Conversation
Two superscripts weren't rendered as such, because the leading (escaped) space before ':sup:`<num>`' was missing. Also, the table of command line arguments was rendered as text, rendering double dashes as em or en dashes (at least in my browser). Finally, removed two extraneous words in the "Initialization by struct" paragraph. Also, why is this .rst, but other parts are .md? I think that leads to a lot of code being simply rendered as italics: anything inside single backticks without "code::" before it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few details to fix up
@@ -63,26 +63,26 @@ Table 5.1: Command-line options for Kokkos::initialize | |||
|
|||
* - Argument | |||
- Description | |||
* - --kokkos-help --help | |||
* - :code:`--kokkos-help --help` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's just my_kokkos_executable --kokkos-help
@@ -100,14 +100,17 @@ The values are case insensitive. | |||
5.2 Initialization by environment variable | |||
------------------------------------------ | |||
|
|||
Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we have | |||
Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we may use | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please link to table 5.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By link, I meant a URL one can click for additional information. Is "< Table clio-pts>" a clickable URL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's supposed to be one, yes. You click it, and it jumps to Table 5.1. For that, I put an anchor above it, in line 60:
.. Table_cli-opts:
I'm not an expert in reStructured text, and neither is GitHub's preview, so references are a bit iffy. But the syntax
`Table 5.1 <Table_cli-opts>`_
is supposed to display 'Table 5.1' and the reference target is what's within the angle braces (< >
). The underscore at the end is necessary, but I don't know, why.
5.3 Initialization by struct | ||
---------------------------- | ||
|
||
Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set a options using the struct, one can use the set functions `set_xxx` where `xxx` is the identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions. | ||
Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set a options using the struct, one can use the functions `set_xxx` where `xxx` is identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments:
- Please link to Table 5.1
- In the "If one wants to set a options using the struct," clause, please remove the "a"; the sentence should read, "If one wants to set options using the struct, one can use the functions
set_xxx
wherexxx
is identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. "
ping @RL-S |
response to kokkos#531 (review) Inserted custom anchor to allow referencing Table 5.1 and used it for all *three* references in the text, removed extraneous 'a' as suggested, and removed `--help` after `--kokkos-help`
Was on holiday until yesterday. See new commit, should resolve it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good once the question around a working link to Table-5.1 is finalized.
@@ -49,40 +49,42 @@ Kokkos chooses the two spaces using the following list: | |||
8. `Kokkos::Threads` | |||
9. `Kokkos::Serial` | |||
|
|||
The highest execution space in the list that is enabled is Kokkos' default execution space, and the highest enabled host execution space is Kokkos' default host execution space. For example, if `Kokkos::Cuda`, `Kokkos::OpenMP`, and `Kokkos::Serial` are enabled, then `Kokkos::Cuda` is the default execution space and `Kokkos::OpenMP` is the default host execution space.:sup:`1` In cases where the highest enabled backend is a host parallel execution space the `DefaultExecutionSpace` and the `DefaultHostExecutionSpace` will be the same. | |||
The highest execution space in the list that is enabled is Kokkos' default execution space, and the highest enabled host execution space is Kokkos' default host execution space. For example, if `Kokkos::Cuda`, `Kokkos::OpenMP`, and `Kokkos::Serial` are enabled, then `Kokkos::Cuda` is the default execution space and `Kokkos::OpenMP` is the default host execution space\ :sup:`1`. In cases where the highest enabled backend is a host parallel execution space the `DefaultExecutionSpace` and the `DefaultHostExecutionSpace` will be the same. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, is the "highest" execution space formally a Kokkos convention, something approximating operator precedence, or is it a concept we just expect everyone to understand automatically? If it's not been formalized, perhaps a sentence or two defining it might be useful ...?
@@ -100,14 +100,17 @@ The values are case insensitive. | |||
5.2 Initialization by environment variable | |||
------------------------------------------ | |||
|
|||
Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we have | |||
Instead of using command-line arguments, one may use environment variables. The environment variables are identical to the arguments in Table 5.1 but they are upper case and the dash is replaced by an underscore. For example, if we want to set the number of threads to 3, we may use | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By link, I meant a URL one can click for additional information. Is "< Table clio-pts>" a clickable URL?
5.3 Initialization by struct | ||
---------------------------- | ||
|
||
Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set a options using the struct, one can use the set functions `set_xxx` where `xxx` is the identical to the arguments in Table 5.1 where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions. | ||
Instead of giving `Kokkos::initialize() <../API/core/initialize_finalize/initialize.html>`_ command-line arguments, one may directly pass in initialization parameters using the `Kokkos::InitializationSettings` struct. If one wants to set options using the struct, one can use the functions `set_xxx` where `xxx` is identical to the arguments in `Table 5.1 <Table_cli-opts>`_ where the dash has been replaced by an underscore. To check if a variable has been set, one can use the `has_xxx` functions. Finally, to get the value that was set, one can use the `get_xxx` functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there API documentation on using the struct for initialization settings? If not, that might be something to do, comparing one method to the other. I have the same question as above about the link to Table 5.1.
Two superscripts weren't rendered as such, because the leading (escaped) space before ':sup:
<num>
' was missing. Also, the table of command line arguments was rendered as text, rendering double dashes as em or en dashes (at least in my browser). Finally, removed two extraneous words in the "Initialization by struct" paragraph. Also, why is this .rst, but other parts are .md? I think that leads to a lot of code being simply rendered as italics: anything inside single backticks without "code::" before it.